freshness ui #29297
freshness ui #29297
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
Deploy preview for dagit-core-storybook ready! ✅ Preview Built with commit 187a8b7. |
jamiedemaria
force-pushed
the
jamie/freshness-health-uis
branch
from
5a7a60e to
56030c0
Compare
|
|
||
| return ( | ||
| <Body> | ||
| Last materialized {dayjs(Number(metadata.lastMaterializedTimestamp)).fromNow()} ago |
There was a problem hiding this comment.
@salazarm i don't know why, but this is giving me output like 55 days ago (see the screenshots in the description) - i checked the GQL query response and the timestamp is correct (example 1744821283.3374982) so i must be doing some incorrect conversion here?
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
|
|
||
| return ( | ||
| <Body> | ||
| Last materialized {dayjs(Number(metadata.lastMaterializedTimestamp)).fromNow()} ago |
There was a problem hiding this comment.
The dayjs.fromNow() method already includes the word "ago" in its output for past dates (e.g., "5 minutes ago"). Adding " ago" after this call creates redundant text like "5 minutes ago ago". Consider removing the trailing " ago" to display the correct time format:
Last materialized {dayjs(Number(metadata.lastMaterializedTimestamp)).fromNow()}| Last materialized {dayjs(Number(metadata.lastMaterializedTimestamp)).fromNow()} ago | |
| Last materialized {dayjs(Number(metadata.lastMaterializedTimestamp)).fromNow()} |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
js_modules/dagster-ui/packages/ui-core/src/assets/AssetHealthSummary.tsx
Show resolved
Hide resolved
|
Deploy preview for dagster-docs ready! Preview available at https://dagster-docs-k3smlj921-elementl.vercel.app Direct link to changed pages: |
| Last materialized {dayjs(Number(metadata.lastMaterializedTimestamp * 1000)).fromNow()}{' '} | ||
| ago |
There was a problem hiding this comment.
There appears to be an issue with the timestamp conversion. The code is multiplying metadata.lastMaterializedTimestamp by 1000 (converting seconds to milliseconds), but dayjs.fromNow() already expects the timestamp to be in the correct format.
If lastMaterializedTimestamp is in Unix seconds (which appears to be the case based on the multiplication), the correct approach would be to use:
dayjs.unix(metadata.lastMaterializedTimestamp).fromNow()This will properly handle the Unix timestamp in seconds without needing manual conversion. Alternatively, if manual conversion is preferred, ensure the conversion is appropriate for how fromNow() expects to receive timestamps.
The current implementation will display times that are off by a factor of 1000, which could lead to confusing user experiences.
| Last materialized {dayjs(Number(metadata.lastMaterializedTimestamp * 1000)).fromNow()}{' '} | |
| ago | |
| Last materialized {dayjs.unix(metadata.lastMaterializedTimestamp).fromNow()}{' '} |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
jamiedemaria
force-pushed
the
jamie/freshness-health-uis
branch
from
13b5659 to
187a8b7
Compare
## Summary & Motivation Adds helper text for freshness health - `NOT_APPLICABLE` - "No freshness policy defined" - `UNKNOWN` - no text - `DEGRADED` - "Last materialized at <timestamp>" or "No materializations" if the asset has never been materialized - `WARNING` - "Last materialized at <timestamp>" or "No materializations" if the asset has never been materialized - `HEALTHY` - no text ## How I Tested These Changes <img width="384" alt="Screenshot 2025-04-16 at 12 12 41 PM" src="https://github.com/user-attachments/assets/ccd18caf-1f20-464b-8bef-61f51338ad88" /> <img width="384" alt="Screenshot 2025-04-16 at 12 13 17 PM" src="https://github.com/user-attachments/assets/ad5edd69-8b11-417e-8988-6d89a2e9749f" /> (ignore the strange time in these screenshots, that has been fixed) <img width="384" alt="Screenshot 2025-04-16 at 12 36 35 PM" src="https://github.com/user-attachments/assets/58b05dce-4a89-462c-b981-28bdda13ba71" /> <img width="384" alt="Screenshot 2025-04-16 at 12 39 52 PM" src="https://github.com/user-attachments/assets/6f16da5e-ca80-44ee-a641-6ba0eb789aaa" /> --------- Co-authored-by: Marco Salazar <[email protected]>
Summary & Motivation
Adds helper text for freshness health
NOT_APPLICABLE- "No freshness policy defined"UNKNOWN- no textDEGRADED- "Last materialized at " or "No materializations" if the asset has never been materializedWARNING- "Last materialized at " or "No materializations" if the asset has never been materializedHEALTHY- no textHow I Tested These Changes
(ignore the strange time in these screenshots, that has been fixed)
Changelog